[WIP] update suggestions for add features in aws mutlti zone support #1325
Conversation
Run daily jobs for multiple versions, each with a different source branch. Also select the worker type based on the branch. Signed-off-by: Pablo Chacin <[email protected]>
Use double quotes in worker label to allow for variable substitution. Signed-off-by: Pablo Chacin <[email protected]>
…titution Fix quotes in label to allow variable substitution
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Signed-off-by: Vicente Zepeda Mas <[email protected]>
Kubernetes 1.17 requires go version 1.13[1]. This change updates the makefile version check and the package spec file template to require go 1.13 and updates the module list with `go mod tidy`. This change will require an update to the Jenkins workers to use 1.13. [1] https://kubernetes.io/docs/setup/release/notes/#dependencies
If ci-repo label is specified, set the BRANCH_REPO environment variable to add this repo branch to the cluster nodes. Signed-off-by: Pablo Chacin <[email protected]>
Separate jobs in different projects depending on the version of CaaSP to facilitate adapting their configuration independently. Signed-off-by: Pablo Chacin <[email protected]>
Following the convention that master and feature branches will use the default integration worker type, while the experimental-feature and maintenance-version branches will use the corresponding worker types. Signed-off-by: Pablo Chacin <[email protected]>
Make v4 run from maintenance-caasp-v4 and v5 from master Signed-off-by: Pablo Chacin <[email protected]>
…t-brach Select worker type based on target branch for PR
We'd like to test the v5 versions of the k8s images.
Require go version 1.13
Upgrade k8s libraries
Adapt terraform tfvars to start using SLE_15_SP2 repositories and CaaSP v5 devel projects
AutoYaST adapted for CaaSP 5 based on SLE-15-SP2
Switch daily job version branches
Switch to v5 CR container images
Collects logs from provisioned nodes including cloud-init and pods. Signed-off-by: Pablo Chacin <[email protected]>
Fixes: f3d2d0b ("Update to Kubernetes v1.18.2") Signed-off-by: Michal Rostecki <[email protected]>
versions: Fix Cilium version for k8s v1.18.2
Fixes: f3d2d0b ("Update to Kubernetes v1.18.2") Signed-off-by: Michal Rostecki <[email protected]> Co-authored-by: Michal Rostecki <[email protected]>
Experimental v5
The files are supposed to be in /etc/crio/crio.conf.d/ not /etc/crio/conf.d/. Also cleanup wording a bit.
Updated versions to v4.5 includes packages and ci
updated version to v4.5
This reverts commit: * 5757a23 * 48f33d6 * 925f938 In 4.2.2 we have reverted k8s to 1.17 prior releasing. So we need to backport this change to master as well. Signed-off-by: Jordi Massaguer Pla <[email protected]>
Revert k8s 1.17.9 upgrade
Signed-off-by: Manuel Buil <[email protected]>
Add the CA repo
Fixes CVE-2020-8663, CVE-2020-12603, CVE-2020-12604, CVE-2020-12605 in cilium.
… authority (SUSE#1262) * Signing OIDC server cert by custom OIDC CA if present * New command: skuba cert generate-csr Signed-off-by: JenTing Hsiao <[email protected]>
Signed-off-by: JenTing Hsiao <[email protected]>
When removing the package zypper will install freely any version it finds more suitable, if there is a new one it will install that one.
After the PR are merged into the target branch before testing. Therefore running the PR tests agains master/release branches is redundant. Signed-off-by: Pablo Chacin <[email protected]>
Revert k8s versin to 1.17.4 as "previous" version for maintenance branch in the upgrade tests Signed-off-by: Pablo Chacin <[email protected]>
Update to cilium 1.7.6 (bsc#1173559)
Lock kured after disabling skuba-update.timer
…ance Fix K8s version in upgrade tests for maintenance
update suggestions from code review for add features in aws mutlti zone support Signed-off-by: cclhsu <[email protected]>
There was a problem hiding this comment.
Hey, Looked over the PR a little bit this morning, had a few quick questions regarding it.
-
could you fill out the PR template a bit more? While i'm a bit fan of "the code should speak for itself" a lot of the time the code does not give a good "why" and that would be helpful!
-
the PR is quite large :D anything you could do to slim it down would make it easier to review
-
generally, I don't think the API server should be exposed externally unless someone absolutely needs it... and lets be honest, no one actually needs it. A large part of the security CVE's can be mitigated by keeping the control plane safe.
| description = "etcd" | ||
| } | ||
|
|
||
| # api-server - everywhere |
There was a problem hiding this comment.
Same here, Api server should be internal only as well.
There was a problem hiding this comment.
var.cidr_block is internal.
| cidr_blocks = ["0.0.0.0/0"] | ||
| } | ||
|
|
||
| ingress { |
There was a problem hiding this comment.
@dannysauer Correct me if i'm wrong but I don't think we should expose the API server too the web. this is an easy first step to securing them, making it so that only inside the cluster can directly hit the api server. I would be for removing this block.
There was a problem hiding this comment.
I'm not completely clear why we open 80/443 to begin with; nothing listens on those ports by default, right? Was that just a copy-paste from an example that was left behind all along?
There was a problem hiding this comment.
Also, yes, exposing the api server seems undesirable. I guess it's possible to control access to the whole vpc and that might be an acceptable control, but I honestly haven't read the entire config to see if that's being done. It still seems like it'd be preferable to elect a source IP as "management node" or identify a management network.
There was a problem hiding this comment.
This is from original code. I just try to rearrange code by function. I will test to remove 80/443 to see if any side effect
cclhsu
changed the title
Ok, I will slim PR down to focus just multiple zone. I am add too many formatting change and little customized improvemnt here and there. |
There was a problem hiding this comment.
I'm in the same boat as @Klaven ; this looks fine to me as a cleanup, but I don't have enough "AWS via Terraform" background to say it's good to go.
update suggestions from code review for add features in aws mutlti zone support
Signed-off-by: cclhsu [email protected]
Why is this PR needed?
Does it fix an issue? addresses a business case?
add a description and link to the issue if one exists.
Fixes #
Reminder: Add the "fixes bsc#XXXX" to the title of the commit so that it will
appear in the changelog.
What does this PR do?
please include a brief "management" technical overview (details are in the code)
Anything else a reviewer needs to know?
Special test cases, manual steps, links to resources or anything else that could be helpful to the reviewer.
Info for QA
This is info for QA so that they can validate this. This is mandatory if this PR fixes a bug.
If this is a new feature, a good description in "What does this PR do" may be enough.
Related info
Info that can be relevant for QA:
Status BEFORE applying the patch
How can we reproduce the issue? How can we see this issue? Please provide the steps and the prove
this issue is not fixed.
Status AFTER applying the patch
How can we validate this issue is fixed? Please provide the steps and the prove this issue is fixed.
Docs
If docs need to be updated, please add a link to a PR to https://github.com/SUSE/doc-caasp.
At the time of creating the issue, this PR can be work in progress (set its title to [WIP]),
but the documentation needs to be finalized before the PR can be merged.
Merge restrictions
(Please do not edit this)
We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises: